Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gui): Smoothening startup Part I #355

Merged
merged 5 commits into from
Oct 31, 2023
Merged

Conversation

CarlosNihelton
Copy link
Contributor

@CarlosNihelton CarlosNihelton commented Oct 23, 2023

I think this renders best if split in two pull requests: one dealing with the lower level code (this one) and another dealing with the visible parts of the user interface. This way it's easier for reviewers and also unblocks @EduardGomezEscandell's work more quickly.

So in this PR we reduce the duration of a timeout in case no changes in the startup state machine happen and we watch for the creation of the addr file. The watch only happens in the code path that causes the GUI to start the agent. For the watch operation to happen we need the directory to exist, so I added a precaution step to create it. It doesn't fail in case the directory already exists. It's important that we start watching the directory before launching the agent, otherwise race conditions could happen. Thus we start watching first, then launch the agent, and finally wait for the watching operation to complete (which may have completed by the time we start waiting, which is not a problem at all). This prevents the GUI from spawning more than one agent process.

The last commit increases the timeout of one specific end-to-end test because I could see it failing before in CI killing the context, although I could never reproduce it locally not even with VMs.

By means of the Filesystem events API.
That ensures we don't launch the agent more than once,
until startup timeouts.
What happens if we start watching for the addr file but
it never shows up?

The answer is: our outer timeout catches us.
So the stream would emit 'AgentState.unreachable'
and we are done.
That was previously reduced.
Speeding up the agent seems to have broken the test.
Copy link
Contributor

@EduardGomezEscandell EduardGomezEscandell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Short but sweet!

I verified on my machine and indeed only one Ubuntu-Pro-Agent spawns :D

end-to-end/organization_token_test.go Show resolved Hide resolved
@CarlosNihelton CarlosNihelton merged commit 6699249 into main Oct 31, 2023
32 of 35 checks passed
@CarlosNihelton CarlosNihelton deleted the smooth-startup-p1 branch October 31, 2023 03:35
CarlosNihelton added a commit that referenced this pull request Oct 31, 2023
This completes UDENG-1595, by building on top of #355 with the visible
aspects of the change.
Now the GUI automatically attempts to delete the `addr` file and restart
the agent.
Here's how it looks like when a port file is found but the agent is not
running, thus not responsive.


https://github.com/canonical/ubuntu-pro-for-windows/assets/11138291/870b32e3-17f6-4890-8376-0473b0cb5b23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants